-
Notifications
You must be signed in to change notification settings - Fork 405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce --disable-entrypoint-overwrite
option to allow to disable entrypoint overwrite
#1138
Introduce --disable-entrypoint-overwrite
option to allow to disable entrypoint overwrite
#1138
Conversation
Thanks for this contribution! I've never used Delve myself, but I'd definitely love to have better support for it, so having someone with that use case is definitely useful. My main question is what should happen if the base image doesn't have an entrypoint when Would it make sense to have an If it's possible I'd also be interested in more first-class support for |
...And in fact that seems to be what #1128 starts to enable! 🎉 If that seems promising we can push on that PR and see if that helps with your use case as well. |
Hello @imjasonh,
IMO this flag should only be set intentionally. If the user uses a base image without an entrypoint and disables the override, we could log a warning (theoretically the app can still be called directly via
I though of this at the beginning too, but then "discarded" the idea to keep the parameter simple and not always have to pass the full entrypoint. Having this in a preconfigured base image seemed for me simpler. But for sure I could update it.
This is a great idea. My only concerns are, that we would need to provide some kind of base image then (IIUC) and I am not sure, if the then provided Delve configuration parameters would fit for most users (e.g. port, should the app start or wait on startup (
IIUC this does not allow to pass parameters for the binary too and then would require the user to specify them via So whatever helps most I would be happy to help and contribute, as I am really looking for some easy way to debug on ko built apps. |
837a757
to
cdcb929
Compare
Putting this on hold, as #1148 might be a better approach |
This Pull Request is stale because it has been open for 90 days with |
Currently the entrypoint of the image is always set to
/ko-app/<app-name>
. This makes it hard to have some kind of wrapper around the app (e.g. in case you want to remote debug the app via Delve, which invokes the app e.g. viadlv exec <path-to-app>
).This PR addresses it and introduces a new option (
--disable-entrypoint-overwrite
), which allows to use the entrypoint of the base image. To be able to get the path to the apps binary, this PR also introduce theKO_APP_PATH
var, which points to the apps binary.That way a developer who wants to remote debug their app (without to adjust yamls with an updated command & arg) could simply use a base image with a pre setup entrypoint, like the following:
and build/apply via: